FEATURE/SN-368 : Workbench module enahancement to have auto localisation creation for mdms data create #2058
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces comprehensive changes across multiple files in the micro-ui project, focusing on enhancing the workbench and MDMS (Master Data Management System) components. The modifications include styling improvements in CSS, updates to UI customizations, refinements in form handling, and localization support. The changes aim to improve the user interface, data management, and overall functionality of the workbench and related modules. Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)Pattern micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)Pattern 🪛 Biome (1.9.4)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js[error] 35-35: This let declares a variable that is only assigned once. 'actionItems' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) [error] 123-123: Template literals are preferred over string concatenation. Unsafe fix: Use a template literal. (lint/style/useTemplate) [error] 177-177: Change to an optional chain. Unsafe fix: Change to an optional chain. (lint/complexity/useOptionalChain) micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js[error] 183-185: A form label must be associated with an input. Consider adding a (lint/a11y/noLabelWithoutControl) [error] 193-195: A form label must be associated with an input. Consider adding a (lint/a11y/noLabelWithoutControl) [error] 249-249: Template literals are preferred over string concatenation. Unsafe fix: Use a template literal. (lint/style/useTemplate) [error] 393-393: Change to an optional chain. Unsafe fix: Change to an optional chain. (lint/complexity/useOptionalChain) [error] 400-400: Change to an optional chain. Unsafe fix: Change to an optional chain. (lint/complexity/useOptionalChain) [error] 419-419: Template literals are preferred over string concatenation. Unsafe fix: Use a template literal. (lint/style/useTemplate) [error] 485-492: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. Unsafe fix: Use a SelfClosingElement instead (lint/style/useSelfClosingElements) [error] 310-310: This let declares a variable that is only assigned once. 'newAdditionalProps' is never reassigned. Safe fix: Use const instead. (lint/style/useConst) 🔇 Additional comments (28)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🔭 Outside diff range comments (8)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Line range hint
66-71: Consider making toast timeout configurableThe toast timeout is hardcoded to 5000ms. Consider making this configurable or using a constant for better maintainability.
+ const TOAST_TIMEOUT_MS = 5000; + const closeToast = () => { setTimeout(() => { setShowToast(null) - }, 5000); + }, TOAST_TIMEOUT_MS); }micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js (3)
Line range hint
24-42: Add validation for required query parameters.The code extracts query parameters but doesn't validate if required parameters are present. Consider adding validation to ensure critical parameters like
masterNameandmoduleNameare provided when needed.let {masterName:modulee,moduleName:master,tenantId} = Digit.Hooks.useQueryParams() let {from, screen, action} = Digit.Hooks.useQueryParams() + + // Validate required parameters when needed + if (!tenantId) { + tenantId = Digit.ULBService.getCurrentTenantId(); + } + + // Validate master and module when searching specific schema + if ((master && !modulee) || (!master && modulee)) { + console.error('Both master and module must be provided together'); + }🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: This let declares a variable that is only assigned once.
'Config' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 26-26: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
Line range hint
43-91: Add error handling for schema search API.The schema search API integration lacks error handling. Consider adding error states and user feedback for API failures.
const { isLoading, data: dropdownData } = Digit.Hooks.useCustomAPIHook({ url: `/${Digit.Hooks.workbench.getMDMSContextPath()}/schema/v1/_search`, params: { }, body: { SchemaDefCriteria }, config: { + onError: (error) => { + // Handle API errors + console.error('Schema search failed:', error); + // Show error notification to user + Digit.Hooks.useToast.error('Failed to fetch schema data'); + }, select: (data) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: This let declares a variable that is only assigned once.
'Config' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 26-26: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
Line range hint
92-150: Externalize column configurations.Column configurations are hard-coded within the component. Consider moving these to a separate configuration file for better maintainability.
+ // Create a new file: columnConfigs.js + export const getColumnConfig = (dropDownOptions) => ({ + defaultColumns: [ + ...dropDownOptions.map(option => ({ + label: option.i18nKey, + i18nKey: option.i18nKey, + jsonPath: `data.${option.code}`, + dontShowNA: true + })), + { + label: "WBH_ISACTIVE", + i18nKey: "WBH_ISACTIVE", + jsonPath: `isActive`, + additionalCustomization: true + } + ] + }); // In MDMSSearchv2.js - Config.sections.searchResult.uiConfig.columns = [...dropDownOptions.map...] + import { getColumnConfig } from './columnConfigs'; + Config.sections.searchResult.uiConfig.columns = getColumnConfig(dropDownOptions).defaultColumns;🧰 Tools
🪛 Biome (1.9.4)
[error] 22-22: This let declares a variable that is only assigned once.
'Config' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 26-26: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js (1)
Line range hint
1-272: Add TypeScript type definitions for configuration objects.The configuration objects lack type definitions, making it harder to maintain and validate the structure. Consider adding TypeScript interfaces or JSDoc type definitions.
+ /** + * @typedef {Object} UIConfig + * @property {Function} customValidationCheck - Validates form data + * @property {Function} preProcess - Prepares request data + * @property {Function} additionalCustomizations - Handles custom rendering + * @property {Function} MobileDetailsOnClick - Handles mobile view clicks + */ + /** + * @type {Object.<string, UIConfig>} + */ export const UICustomizations = { // ... existing code };micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (3)
Line range hint
714-737: Improve validation message handling in customValidationCheckThe validation message for missing locale is hardcoded. Consider using a more generic validation message key that follows the existing pattern in the codebase.
customValidationCheck: (data) => { const { locale } = data; if (locale === "") - return { warning: true, label: "WBH_LOC_WARNING_LOCALE_MUST_BE_PRESENT" }; + return { warning: true, label: "ES_COMMON_REQUIRED_FIELD" }; return false; }
Line range hint
738-762: Enhance parameter cleanup in preProcess functionThe current implementation deletes parameters one by one. Consider using object destructuring for cleaner code.
preProcess: (data, additionalDetails) => { delete data.body.custom const tenant = Digit.ULBService.getStateId(); - const { locale = undefined, module: modulee = undefined, codes = undefined, message = undefined } = data.params - delete data.params.locale - delete data.params.module - delete data.params.codes - delete data.params.message + const { locale, module: modulee, codes, message, ...restParams } = data.params; + data.params = { + ...restParams, + tenantId: tenant, + ...(locale && { locale: locale.value }), + ...(modulee && { module: modulee.value }), + ...(codes && { codes }) + }; - data.params.tenantId = tenant - if (locale) { - data.params.locale = locale.value - } - if (modulee) { - data.params.module = modulee.value - } - if (codes) { - data.params.codes = codes - } return data; }
Line range hint
763-795: Simplify additionalCustomizations switch statementThe function contains several unused cases from copied code. Clean up the switch statement to only include relevant cases.
additionalCustomizations: (row, key, column, value, t, searchResult) => { switch (key) { - case "Unique Identifier": - const [moduleName, masterName] = row.schemaCode.split(".") - return ( - <span className="link"> - <Link to={`/${window.contextPath}/employee/workbench/mdms-view?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${row.uniqueIdentifier}`}> - {String(value ? (column.translate ? t(column.prefix ? `${column.prefix}${value}` : value) : value) : t("ES_COMMON_NA"))} - </Link> - </span> - ); - case "MASTERS_SOCIAL_CATEGORY": - case "CORE_COMMON_PROFILE_CITY": - case "MASTERS_WARD": - case "MASTERS_LOCALITY": default: return t("ES_COMMON_NA"); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (4)
micro-ui/web/core/inter-package.jsonis excluded by!**/*.jsonmicro-ui/web/micro-ui-internals/package.jsonis excluded by!**/*.jsonmicro-ui/web/sandbox/inter-package.jsonis excluded by!**/*.jsonmicro-ui/web/workbench/inter-package.jsonis excluded by!**/*.json
📒 Files selected for processing (9)
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js(5 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js(3 hunks)micro-ui/web/public/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSSearchv2.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/sandbox/src/configs/UICustomizations.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 178-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 200-208: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 209-209: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 154-154: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 163-163: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 179-179: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 162-164: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 172-174: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 230-230: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 324-324: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 338-338: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 366-366: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 372-372: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 391-391: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 457-464: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 291-291: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 320-320: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
[error] 136-136: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 123-123: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 130-130: This let declares a variable that is only assigned once.
'locModuleName' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 131-131: This let declares a variable that is only assigned once.
'transformedModuleName' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 138-138: This let declares a variable that is only assigned once.
'transformedLocCode' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
[error] 140-140: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 82-82: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (15)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (3)
Line range hint 1-214: Implementation aligns well with PR objectives
The code successfully implements localization support for the MDMS module as intended. While there are some minor improvement opportunities noted above, the overall implementation is solid and achieves the stated goals.
🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 178-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 200-208: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 209-209: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 154-154: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 163-163: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 179-179: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
149-152: 🧹 Nitpick (assertive)
Use optional chaining and const declarations
Improve code safety and maintainability with optional chaining and proper constant declarations.
- let localizationCodes = [];
+ const localizationCodes = [];
- if (data && data.data && localisableFields.length > 0) {
+ if (data?.data && localisableFields?.length > 0) {
localizationCodes = localisableFields.map(field => createLocalizationCode(field.fieldPath, data.data[field.fieldPath]));
}Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
15-15: Verify tenant ID override implications
The tenant ID from query params is being overridden with getCurrentTenantId(). Ensure this doesn't break any functionality that might depend on the query param tenant ID.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (4)
17-19: Confirm timing for toast closure.
You are deferring the toast closure by 5 seconds, which might be too long or too short depending on your UX requirements. Ensure this aligns well with your design guidelines.
37-38: Validate presence of uniqueIdentifier.
The current request references “uniqueIdentifiers: [uniqueIdentifier]”. Verify you always have a valid uniqueIdentifier in your query parameters; an undefined value may cause errors or empty results from the MDMS.
104-115: Ensure safe usage of cloneDeep.
lodash.cloneDeep is powerful but can be expensive on large objects. If your data is large or performance-critical, consider narrowing the cloned portion to only the relevant fields.
[performance]
139-139: Prior feedback from jagankumar-egov
@jagankumar-egov has previously recommended not hardcoding or mixing module-specific references here (line 67 in older commits). Ensure that logic does not reintroduce the same issue.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (2)
76-76: Confirm fallback usage for API url.
You’re using a fallback “Digit.Utils.workbench.getMDMSActionURL” if “api?.url” isn’t defined. Ensure that repeating or conflicting logic is not present in other parts of your code triggering unintended requests.
145-145: Duplicate prior comment on locale naming format.
@jagankumar-egov commented on not hardcoding locale or module naming logic. Ensure that the approach used here aligns with that guidance.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (3)
25-26: Great approach for storing additional properties context.
Using a React context for additional properties is clean and fosters separation of concerns, enabling child components to easily access or update relevant data.
366-366: Optional chaining usage.
Consider if the suggested shift to optional chaining in line 366 is relevant or beneficial. Evaluate carefully to ensure you handle all edge cases that an optional chain implies.
🧰 Tools
🪛 Biome (1.9.4)
[error] 366-366: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
376-377: Commendable approach to context usage.
Wrapping the form with AdditionalPropertiesContext ensures your entire form shares consistent state without complex prop threading. Good use of React contexts!
micro-ui/web/public/index.html (1)
10-10: Verify new CSS version.
Ensure that the @egovernments/digit-ui-css v1.8.2-beta.49 has no breaking changes. Validate styles remain consistent.
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss (1)
1054-1088: LGTM! Well-structured CSS with good practices.
The new code-related styling classes are well-organized, follow BEM-like naming conventions, and implement responsive design principles. The use of consistent spacing and color variables enhances maintainability.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js (1)
813-813: LGTM: Configuration object added at the correct location
The SearchLocalisationConfig object is appropriately placed within the UICustomizations object.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
Outdated
Show resolved
Hide resolved
| const additionalParamString = new URLSearchParams(additionalParams).toString(); | ||
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&"+additionalParamString : ""}`) | ||
| } | ||
| //action===DISABLE || ENABLE call update api and show toast respectively | ||
| else{ | ||
| //call update mutation | ||
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`) | ||
| } else { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use template literals for URL construction
Replace string concatenation with template literals for better readability.
- history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`)
+ history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? `&${additionalParamString}` : ''}`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const additionalParamString = new URLSearchParams(additionalParams).toString(); | |
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&"+additionalParamString : ""}`) | |
| } | |
| //action===DISABLE || ENABLE call update api and show toast respectively | |
| else{ | |
| //call update mutation | |
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`) | |
| } else { | |
| const additionalParamString = new URLSearchParams(additionalParams).toString(); | |
| history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? `&${additionalParamString}` : ''}`) | |
| } else { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
| let { moduleName, masterName, tenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams(); | ||
| let { from, screen, action } = Digit.Hooks.useQueryParams() |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const for variables that aren't reassigned
These destructured variables are never reassigned and should use const instead of let for better code maintainability and to prevent accidental reassignment.
- let { moduleName, masterName, tenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams();
- let { from, screen, action } = Digit.Hooks.useQueryParams()
+ const { moduleName, masterName, tenantId: queryTenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams();
+ const { from, screen, action } = Digit.Hooks.useQueryParams()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let { moduleName, masterName, tenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams(); | |
| let { from, screen, action } = Digit.Hooks.useQueryParams() | |
| const { moduleName, masterName, tenantId: queryTenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams(); | |
| const { from, screen, action } = Digit.Hooks.useQueryParams() |
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| let actionItems = [{ | ||
| action:"EDIT", | ||
| label:"Edit Master" | ||
| action: "EDIT", | ||
| label: "Edit Master" | ||
| }] | ||
|
|
||
| const isActive = data?.isActive | ||
| if(isActive) actionItems.push({ | ||
| action:"DISABLE", | ||
| label:"Disable Master" | ||
| }) | ||
| else actionItems.push({ | ||
| action:"ENABLE", | ||
| label:"Enable Master" | ||
| }) | ||
| if (isActive) { | ||
| actionItems.push({ action: "DISABLE", label: "Disable Master" }) | ||
| } else { | ||
| actionItems.push({ action: "ENABLE", label: "Enable Master" }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve action items initialization
The action items array uses let but is never reassigned. Also, consider extracting the hardcoded action labels for better maintainability.
+ const ACTION_LABELS = {
+ EDIT: "Edit Master",
+ DISABLE: "Disable Master",
+ ENABLE: "Enable Master"
+ };
+
const fetchActionItems = (data) => {
- let actionItems = [{
+ const actionItems = [{
action: "EDIT",
- label: "Edit Master"
+ label: ACTION_LABELS.EDIT
}]
const isActive = data?.isActive
if (isActive) {
- actionItems.push({ action: "DISABLE", label: "Disable Master" })
+ actionItems.push({ action: "DISABLE", label: ACTION_LABELS.DISABLE })
} else {
- actionItems.push({ action: "ENABLE", label: "Enable Master" })
+ actionItems.push({ action: "ENABLE", label: ACTION_LABELS.ENABLE })
}
return actionItems
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let actionItems = [{ | |
| action:"EDIT", | |
| label:"Edit Master" | |
| action: "EDIT", | |
| label: "Edit Master" | |
| }] | |
| const isActive = data?.isActive | |
| if(isActive) actionItems.push({ | |
| action:"DISABLE", | |
| label:"Disable Master" | |
| }) | |
| else actionItems.push({ | |
| action:"ENABLE", | |
| label:"Enable Master" | |
| }) | |
| if (isActive) { | |
| actionItems.push({ action: "DISABLE", label: "Disable Master" }) | |
| } else { | |
| actionItems.push({ action: "ENABLE", label: "Enable Master" }) | |
| } | |
| const ACTION_LABELS = { | |
| EDIT: "Edit Master", | |
| DISABLE: "Disable Master", | |
| ENABLE: "Enable Master" | |
| }; | |
| const actionItems = [{ | |
| action: "EDIT", | |
| label: ACTION_LABELS.EDIT | |
| }] | |
| const isActive = data?.isActive | |
| if (isActive) { | |
| actionItems.push({ action: "DISABLE", label: ACTION_LABELS.DISABLE }) | |
| } else { | |
| actionItems.push({ action: "ENABLE", label: ACTION_LABELS.ENABLE }) | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| const transformedFormData = { ...formData }; | ||
|
|
||
| for (const fieldName in additionalProperties) { | ||
| if (additionalProperties.hasOwnProperty(fieldName)) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace “.hasOwnProperty” with “Object.hasOwn()”.
Consistent with best practices.
- if (additionalProperties.hasOwnProperty(fieldName)) {
+ if (Object.hasOwn(additionalProperties, fieldName)) {
- if (additionalProperties.hasOwnProperty(fieldName)) {
+ if (Object.hasOwn(additionalProperties, fieldName)) {Also applies to: 338-338
🧰 Tools
🪛 Biome (1.9.4)
[error] 324-324: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
| <Toast | ||
| label={t(showToast)} | ||
| error={showErrorToast} | ||
| onClose={() => { | ||
| setShowToast(null); | ||
| }} | ||
| isDleteBtn={true} | ||
| ></Toast> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional self-closing JSX.
For readability, self-close the Toast if it has no children.
🧰 Tools
🪛 Biome (1.9.4)
[error] 457-464: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
| const secondFormatLocalizationMutation = Digit.Hooks.useCustomAPIMutationHook(reqCriteriaSecondUpsert); | ||
|
|
||
| const onSubmitV2 = async ({ formData }, e) => { | ||
| let locale = Digit.SessionStorage.get("locale") || Digit.Utils.getDefaultLanguage(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const for locale.
This variable holding locale is never reassigned; switching to const reduces the chance of accidental reassignments.
- let locale = Digit.SessionStorage.get("locale") || Digit.Utils.getDefaultLanguage();
+ const locale = Digit.SessionStorage.get("locale") || Digit.Utils.getDefaultLanguage();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let locale = Digit.SessionStorage.get("locale") || Digit.Utils.getDefaultLanguage(); | |
| const locale = Digit.SessionStorage.get("locale") || Digit.Utils.getDefaultLanguage(); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 320-320: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
| <Card className="workbench-create-form"> | ||
| <Header className="digit-form-composer-sub-header"> | ||
| {typeof Digit?.Utils?.workbench?.getMDMSLabel === "function" | ||
| ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_` + schema?.code)) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Prefer template literals.
Static analysis recommends a template literal for “SCHEMA_ + schema?.code”. Update it for consistency and clarity.
- : `SCHEMA_${schema?.code}`}
+ : `SCHEMA_${schema?.code}`}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 391-391: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/configs/UICustomizations.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 10
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js(6 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js(3 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
[error] 124-124: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js
[error] 8-8: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 150-150: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 178-178: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 200-208: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 209-209: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 154-154: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 163-163: This let declares a variable that is only assigned once.
'messageMap' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 179-179: This let declares a variable that is only assigned once.
'updatedData' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 162-164: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 172-174: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 230-230: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 322-322: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 340-340: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 374-374: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 381-381: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 400-400: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 466-473: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 291-291: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 358-358: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
[error] 82-82: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (30)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js (5)
9-29: Looks good for the base structure and navigation.
All initial states and navigation helpers (gotoView, closeToast) appear logically consistent, and no immediate improvements are required.
31-44: MDMS data fetch and schema handling are clear.
The approach of dynamically enabling API calls based on moduleName/masterName presence is appropriate, and the logic for retrieving unique fields to set them read-only is straightforward and correct.
Also applies to: 48-73
75-78: Consider renaming “tranformLocModuleName” to “transformLocModuleName.”
A previous reviewer recommended normalizing the spelling for improved clarity.
131-170: Robust handleUpdate logic.
The error handling and dual-upsert flow (localization + MDMS update) comprehensively address failure scenarios. The user is effectively notified via toasts, and updates are neatly separated.
82-82: 🧹 Nitpick (assertive)
Use const instead of let for “locale.”
This variable is never reassigned.
Apply this diff to fix:
- let locale=Digit.StoreData.getCurrentLanguage();
+ const locale=Digit.StoreData.getCurrentLanguage();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 82-82: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (8)
162-164: Associate label with input for accessibility
This comment has been stated previously.
🧰 Tools
🪛 Biome (1.9.4)
[error] 162-164: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
172-174: Associate label with input for accessibility
This comment has been stated previously.
🧰 Tools
🪛 Biome (1.9.4)
[error] 172-174: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
291-291: Use const for newAdditionalProps
This was recommended in a past review.
🧰 Tools
🪛 Biome (1.9.4)
[error] 291-291: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
322-322: Use Object.hasOwn() instead of .hasOwnProperty
This comment has been stated previously.
🧰 Tools
🪛 Biome (1.9.4)
[error] 322-322: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
340-340: Use Object.hasOwn() instead of .hasOwnProperty
This comment has been stated previously.
🧰 Tools
🪛 Biome (1.9.4)
[error] 340-340: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
400-400: Prefer template literals
This point has been raised before.
🧰 Tools
🪛 Biome (1.9.4)
[error] 400-400: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
466-473: Convert the Toast component to self-closing
This comment has been stated previously.
🧰 Tools
🪛 Biome (1.9.4)
[error] 466-473: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
358-358: 🧹 Nitpick (assertive)
Prefer const for the locale variable
Since “locale” is never reassigned, use const for improved immutability.
- let locale = Digit.StoreData.getCurrentLanguage();
+ const locale = Digit.StoreData.getCurrentLanguage();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 358-358: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (7)
129-133: Logic for retrieving localisable fields looks good
The approach to filter the UISchema based on module & master names is clear. No issues found here.
137-147: Helper functions for localization are well-structured
The functions tranformLocModuleName and createLocalizationCode are concise and readable.
200-208: Use self-closing tag for the button
This mirrors a previous comment about JSX elements without children.
- ></Button>
+ />🧰 Tools
🪛 Biome (1.9.4)
[error] 200-208: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
209-209: Use self-closing tag for the Toast component
This is also a duplicate comment from a previous review. JSX elements without children should be marked as self-closing.
- {showToast && <Toast label={t(showToast.label)} error={showToast?.isError} onClose={() => setShowToast(null)}></Toast>}
+ {showToast && <Toast label={t(showToast.label)} error={showToast?.isError} onClose={() => setShowToast(null)} />}🧰 Tools
🪛 Biome (1.9.4)
[error] 209-209: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
120-124: 🧹 Nitpick (assertive)
Use template literals for better readability
Instead of string concatenation, a template literal will make the code more readable and less error-prone.
- history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? "&" + additionalParamString : ""}`)
+ history.push(`/${window?.contextPath}/employee/workbench/mdms-edit?moduleName=${moduleName}&masterName=${masterName}&uniqueIdentifier=${uniqueIdentifier}${additionalParamString ? `&${additionalParamString}` : ""}`)Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
13-14: 🧹 Nitpick (assertive)
Use const for variables that are never reassigned
The static analysis tool indicates that 'from', 'screen', and 'action' are never reassigned. Replacing 'let' with 'const' can help prevent accidental reassignments and improve maintainability.
- let { from, screen, action } = Digit.Hooks.useQueryParams()
+ const { from, screen, action } = Digit.Hooks.useQueryParams()Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
33-45: 🧹 Nitpick (assertive)
Use const instead of let when array is not reassigned
'actionItems' is never reassigned. Consider using const for better readability and to prevent accidental reassignments.
- let actionItems = [{
+ const actionItems = [{
action: "EDIT",
label: "Edit Master"
}]Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js (10)
8-8: Modular approach for localization import looks good.
Bringing in “buildLocalizationMessages” from a dedicated utility neatly separates concerns, improving maintainability.
68-75: Review the default MDMS object structure.
The body object implies a refined approach to loading the schema code and data. Ensure consistency with the structure documented in your domain’s MDMS specifications (e.g., “schemaCode”, “data”, etc.). If this structure is non-standard, consider adding descriptive comments.
77-77: Confirm usage of fallback URL.
You’re using a dynamic URL from “api?.url” or a fallback from “Digit.Utils.workbench…”. Verify that both references are correct to avoid misdirecting requests.
100-110: Localization upsert mutation is properly defined.
Good job creating a separate mutation for handling localization, which helps keep the logic well-organized.
111-115: Toast closing approach is valid.
Automatically hiding the toast after a few seconds is fine. Just ensure that users have enough time to read any error or success messages.
118-121: Minor function name mismatch (duplicate comment).
This was noted previously: you use “tranformLocModuleName” here, but “transformLocModuleName” was suggested in other comments. Consider standardizing the name for consistency.
165-171: No localization messages scenario handled well.
Your fallback handles the case of an empty localization workload gracefully, avoiding an unnecessary network call.
223-223: Re-check dynamic properties assignment.
After retrieving the “x-ref-schema,” you update certain properties and reassign to “formSchema.” Confirm all references are consistent with the new structure to prevent runtime errors.
259-259: Clear naming for UI schema object.
“uiJSONSchema” is well-labeled, improving clarity. Nicely done.
123-124: 🧹 Nitpick (assertive)
Use “const” for locale.
According to static analysis, the “locale” variable is never reassigned. You can switch from let to const for clarity.
- let locale = Digit.StoreData.getCurrentLanguage();
+ const locale = Digit.StoreData.getCurrentLanguage();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 124-124: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSEdit.js
Show resolved
Hide resolved
| console.error("Second format localization upsert failed:", err); | ||
| } | ||
| } | ||
| onSubmit && onSubmit(transformedFormData, additionalProperties); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider optional chaining for onSubmit
Using optional chaining is more concise and idiomatic.
- onSubmit && onSubmit(transformedFormData, additionalProperties);
+ onSubmit?.(transformedFormData, additionalProperties);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onSubmit && onSubmit(transformedFormData, additionalProperties); | |
| onSubmit?.(transformedFormData, additionalProperties); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 374-374: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
Outdated
Show resolved
Hide resolved
| let {from, screen, action} = Digit.Hooks.useQueryParams() | ||
| let { moduleName, masterName, tenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams(); | ||
| let { from, screen, action } = Digit.Hooks.useQueryParams() | ||
| tenantId = Digit.ULBService.getCurrentTenantId(); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid reassigning function-scoped variable
Overwriting the destructured tenantId may lead to confusion and potential issues in future code changes. Consider separating them by using a different name for the destructured variable or storing the updated value in a separate constant.
- let { moduleName, masterName, tenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams();
+ const { moduleName, masterName, tenantId: queryTenantId, uniqueIdentifier } = Digit.Hooks.useQueryParams();
// Later in the code:
- tenantId = Digit.ULBService.getCurrentTenantId();
+ const currentTenantId = Digit.ULBService.getCurrentTenantId();Committable suggestion skipped: line range outside the PR's diff.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSAddV2.js
Show resolved
Hide resolved
.../web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
micro-ui/web/micro-ui-internals/packages/css/package.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js(3 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js (1)
Pattern **/*.js: check
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 35-35: This let declares a variable that is only assigned once.
'actionItems' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 177-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/pages/employee/MDMSView.js (5)
14-14: Use const for variables that aren't reassigned
This is a duplicate of a past comment recommending the same improvement.
🧰 Tools
🪛 Biome (1.9.4)
[error] 14-14: This let declares some variables that are only assigned once.
'from' is never reassigned.
'screen' is never reassigned.
'action' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
15-15: Avoid reassigning function-scoped variable
This is a duplicate of a past comment recommending using a separately named constant instead of reusing tenantId.
33-33: Use const for 'actionItems'
This is a duplicate of a past comment recommending the same improvement.
123-123: Use template literals for URL construction
This is a duplicate of a past comment recommending the same improvement.
🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
177-177: Use optional chaining
This is a duplicate of a past comment recommending the same improvement.
🧰 Tools
🪛 Biome (1.9.4)
[error] 177-177: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| export const buildLocalizationMessages = (additionalProperties, moduleName, locale) => { | ||
| if (!additionalProperties || typeof additionalProperties !== "object") return []; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding more specific input validation.
While the function checks if additionalProperties is an object, it could benefit from more specific validation of the expected shape and required parameters.
export const buildLocalizationMessages = (additionalProperties, moduleName, locale) => {
- if (!additionalProperties || typeof additionalProperties !== "object") return [];
+ if (!additionalProperties || typeof additionalProperties !== "object") return [];
+ if (!moduleName || typeof moduleName !== "string") return [];
+ if (!locale || typeof locale !== "string") return [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const buildLocalizationMessages = (additionalProperties, moduleName, locale) => { | |
| if (!additionalProperties || typeof additionalProperties !== "object") return []; | |
| export const buildLocalizationMessages = (additionalProperties, moduleName, locale) => { | |
| if (!additionalProperties || typeof additionalProperties !== "object") return []; | |
| if (!moduleName || typeof moduleName !== "string") return []; | |
| if (!locale || typeof locale !== "string") return []; |
.../web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js
Show resolved
Hide resolved
.../web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js
Outdated
Show resolved
Hide resolved
.../web/micro-ui-internals/packages/modules/workbench/src/pages/employee/localizationUtility.js
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/public/index.html(1 hunks)
🔇 Additional comments (1)
micro-ui/web/public/index.html (1)
10-11: Ensure consistent versions across CSS packages
Both digit-ui-css and digit-ui-components-css packages should use the same version (1.8.11).
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
| const { data: MdmsRes } = Digit.Hooks.useCustomMDMS( | ||
| tenantId, | ||
| "Workbench", | ||
| [{ name: "UISchema" }], |
There was a problem hiding this comment.
use mdms v2 and fetch only required schma not all
There was a problem hiding this comment.
@mithunhegde-egov this is not handled yet ?
There was a problem hiding this comment.
it will cause issues like
i opened pgr , for pgr it works properly
when i open some other schema, since filter is not passed that schema also have this ui localisation config,
so some issue might happen
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss(1 hunks)micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 178-180: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 188-190: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 244-244: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 336-336: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 354-354: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 388-388: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 395-395: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 414-414: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 480-487: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 305-305: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 372-372: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (15)
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss (1)
1050-1051: 🧹 Nitpick (assertive)
Consider using consistent indentation in nested selectors.
The closing brace for the .audit-history class is not properly indented, which affects code readability.
.audit-history {
.diff {
// ... existing styles ...
}
-}
+ }Likely invalid or redundant comment.
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (14)
24-24: Maintain naming consistency.
The function name “tranformLocModuleName” might be a misspelling of “transformLocModuleName.”
47-49: No issues found in these comment lines.
59-59: No further comments.
106-106: No further comments.
116-116: No further comments.
122-122: No further comments.
158-159: No issues found.
388-388: Use optional chaining for onSubmit calls.
Replacing logical AND checks with optional chaining is more concise and idiomatic.
- onSubmit && onSubmit(transformedFormData, additionalProperties);
+ onSubmit?.(transformedFormData, additionalProperties);🧰 Tools
🪛 Biome (1.9.4)
[error] 388-388: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
414-414: Switch to template literals.
- ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_` + schema?.code))
+ ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_${schema?.code}`))🧰 Tools
🪛 Biome (1.9.4)
[error] 414-414: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
480-487: Self-close the Toast.
When a JSX element has no children, using a self-closing tag keeps the structure concise.
- <Toast
- label={t(showToast)}
- error={showErrorToast}
- onClose={() => {
- setShowToast(null);
- }}
- isDleteBtn={true}
- ></Toast>
+ <Toast
+ label={t(showToast)}
+ error={showErrorToast}
+ onClose={() => setShowToast(null)}
+ isDleteBtn={true}
+ />🧰 Tools
🪛 Biome (1.9.4)
[error] 480-487: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
372-372: 🧹 Nitpick (assertive)
Use const for immutable variables.
This variable is never reassigned, so declaring it with const is safer and more consistent.
- let locale = Digit.StoreData.getCurrentLanguage();
+ const locale = Digit.StoreData.getCurrentLanguage();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 372-372: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
305-305: 🧹 Nitpick (assertive)
Use const instead of let for variables that are never reassigned.
- let newAdditionalProps = {};
+ const newAdditionalProps = {};Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 305-305: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
395-395: 🧹 Nitpick (assertive)
Use optional chaining for onFormError calls.
- onFormError && onFormError(errors);
+ onFormError?.(errors);Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 395-395: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
245-245: 🧹 Nitpick (assertive)
Use template literals for dynamic IDs.
Replace string concatenation with a template literal for clearer, more idiomatic code.
- id={"label_" + id}
+ id={`label_${id}`}Likely invalid or redundant comment.
micro-ui/web/micro-ui-internals/packages/css/src/digitv2/pages/employee/workbench.scss
Show resolved
Hide resolved
| padding: 0.5rem 1.5rem; | ||
| background-color: #f9f9f9; | ||
| border: 0.125rem solid #e0e0e0; | ||
| border-radius: 0.25rem; | ||
| } | ||
|
|
||
| .code-row { | ||
| display: flex; | ||
| align-items: flex-start; | ||
| margin-bottom: 0.75rem; | ||
|
|
||
| &:last-child { | ||
| margin-bottom: 0; | ||
| } | ||
| } | ||
|
|
||
| .code-key { | ||
| flex: 0 0 9rem; | ||
| font-size: 0.9rem; | ||
| font-weight: bold; | ||
| color: #444; | ||
| text-align: left; | ||
| line-height: 1.2rem; | ||
| } | ||
|
|
||
| .code-value-container { | ||
| display: flex; | ||
| flex-direction: column; | ||
| flex: 1; | ||
| margin-left: 0.5rem; | ||
| align-items: flex-start; | ||
| } | ||
|
|
||
| .code-value { | ||
| font-size: 0.9rem; | ||
| color: #555; | ||
| line-height: 1.2rem; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM! Well-structured component styles with good organization.
The code details component styles are well-organized with a clear hierarchy and meaningful class names. The use of rem units for measurements and semantic color values is consistent with best practices.
A few suggestions to enhance maintainability:
- Consider extracting color values into SCSS variables
- Add comments to document the component's structure
+ // Code details component - Displays key-value pairs in a structured layout
.code-details {
padding: 0.5rem 1.5rem;
- background-color: #f9f9f9;
- border: 0.125rem solid #e0e0e0;
+ background-color: var(--code-details-bg, #f9f9f9);
+ border: 0.125rem solid var(--code-details-border, #e0e0e0);
border-radius: 0.25rem;
}
.code-row {
display: flex;
align-items: flex-start;
margin-bottom: 0.75rem;
&:last-child {
margin-bottom: 0;
}
}
.code-key {
flex: 0 0 9rem;
font-size: 0.9rem;
font-weight: bold;
- color: #444;
+ color: var(--code-key-color, #444);
text-align: left;
line-height: 1.2rem;
}
.code-value {
font-size: 0.9rem;
- color: #555;
+ color: var(--code-value-color, #555);
line-height: 1.2rem;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .code-details { | |
| padding: 0.5rem 1.5rem; | |
| background-color: #f9f9f9; | |
| border: 0.125rem solid #e0e0e0; | |
| border-radius: 0.25rem; | |
| } | |
| .code-row { | |
| display: flex; | |
| align-items: flex-start; | |
| margin-bottom: 0.75rem; | |
| &:last-child { | |
| margin-bottom: 0; | |
| } | |
| } | |
| .code-key { | |
| flex: 0 0 9rem; | |
| font-size: 0.9rem; | |
| font-weight: bold; | |
| color: #444; | |
| text-align: left; | |
| line-height: 1.2rem; | |
| } | |
| .code-value-container { | |
| display: flex; | |
| flex-direction: column; | |
| flex: 1; | |
| margin-left: 0.5rem; | |
| align-items: flex-start; | |
| } | |
| .code-value { | |
| font-size: 0.9rem; | |
| color: #555; | |
| line-height: 1.2rem; | |
| } | |
| // Code details component - Displays key-value pairs in a structured layout | |
| .code-details { | |
| padding: 0.5rem 1.5rem; | |
| background-color: var(--code-details-bg, #f9f9f9); | |
| border: 0.125rem solid var(--code-details-border, #e0e0e0); | |
| border-radius: 0.25rem; | |
| } | |
| .code-row { | |
| display: flex; | |
| align-items: flex-start; | |
| margin-bottom: 0.75rem; | |
| &:last-child { | |
| margin-bottom: 0; | |
| } | |
| } | |
| .code-key { | |
| flex: 0 0 9rem; | |
| font-size: 0.9rem; | |
| font-weight: bold; | |
| color: var(--code-key-color, #444); | |
| text-align: left; | |
| line-height: 1.2rem; | |
| } | |
| .code-value-container { | |
| display: flex; | |
| flex-direction: column; | |
| flex: 1; | |
| margin-left: 0.5rem; | |
| align-items: flex-start; | |
| } | |
| .code-value { | |
| font-size: 0.9rem; | |
| color: var(--code-value-color, #555); | |
| line-height: 1.2rem; | |
| } |
| <label className="code-key"> | ||
| {t("MDMS_CODE_WORKBENCH")}: | ||
| </label> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Associate labels with inputs or convert them into headings.
Currently, these label elements aren’t linked to any form controls. Consider using a heading element or adding an explicit “htmlFor” that references an input’s id for improved accessibility.
Also applies to: 188-190
🧰 Tools
🪛 Biome (1.9.4)
[error] 178-180: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 177-179: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 187-189: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 243-243: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 338-338: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 356-356: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 390-390: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 397-397: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 416-416: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 482-489: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 307-307: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 374-374: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (7)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (7)
374-374: 🧹 Nitpick (assertive)
Use const for locale.
It is never reassigned.
- let locale = Digit.StoreData.getCurrentLanguage();
+ const locale = Digit.StoreData.getCurrentLanguage();Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 374-374: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
307-307: 🧹 Nitpick (assertive)
Use const instead of let for newAdditionalProps.
This variable is never reassigned.
- let newAdditionalProps = {};
+ const newAdditionalProps = {};Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 307-307: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
416-416: 🧹 Nitpick (assertive)
Use template literals.
Static analysis recommends “SCHEMA_${schema?.code}” for clarity.
- ? t(Digit.Utils.workbench.getMDMSLabel("SCHEMA_" + schema?.code))
+ ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_${schema?.code}`))Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 416-416: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
482-490: 🧹 Nitpick (assertive)
Self-close the Toast component.
Helps maintain a cleaner JSX structure when no children are present.
- <Toast
- label={t(showToast)}
- error={showErrorToast}
- onClose={() => {
- setShowToast(null);
- }}
- isDleteBtn={true}
- ></Toast>
+ <Toast
+ label={t(showToast)}
+ error={showErrorToast}
+ onClose={() => setShowToast(null)}
+ isDleteBtn={true}
+ />Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 482-489: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
390-390: 🧹 Nitpick (assertive)
Use optional chaining.
OnSubmit is conditionally invoked.
- onSubmit && onSubmit(transformedFormData, additionalProperties);
+ onSubmit?.(transformedFormData, additionalProperties);Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 390-390: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
177-179: 🛠️ Refactor suggestion
Associate label with an input.
A label must be linked to a form control via htmlFor or wrapped around the control for accessibility compliance.
- <label className="code-key">
+ <label className="code-key" htmlFor={`mdms-code-${fieldName}`}>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 177-179: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
187-189: 🛠️ Refactor suggestion
Link this label to the corresponding input.
Improve accessibility by referencing the relevant input’s id.
- <label className="code-key">
+ <label className="code-key" htmlFor={`localization-code-${fieldName}`}>Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 187-189: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
| const AdditionalPropertiesContext = createContext(); | ||
| export const useAdditionalProperties = () => useContext(AdditionalPropertiesContext); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add a docstring for context usage.
Explanatory comments for AdditionalPropertiesContext and its hook can help teams quickly understand its purpose and usage.
| const transformFormDataWithProperties = (formData, additionalProperties) => { | ||
| const transformedFormData = { ...formData }; | ||
| for (const fieldName in additionalProperties) { | ||
| if (additionalProperties.hasOwnProperty(fieldName)) { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid calling hasOwnProperty directly on objects.
Use “Object.hasOwn(…)” to ensure reliability and clarity.
- if (additionalProperties.hasOwnProperty(fieldName)) {
+ if (Object.hasOwn(additionalProperties, fieldName)) {Also applies to: 356-356
🧰 Tools
🪛 Biome (1.9.4)
[error] 338-338: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 177-179: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 187-189: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 243-243: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 335-335: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 353-353: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 387-387: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 394-394: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 413-413: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 479-486: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 304-304: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 371-371: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (31)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (31)
14-14: No concerns with this import.
All dependencies are relevant and appear valid.
24-24: Import usage looks good.
No issues with referencingtranformLocModuleName.
26-27: Context creation and hook export look fine.
Implementation aligns with React best practices.
59-59: No changes needed here.
This closing brace is part of existing logic.
106-106: No issues in TitleFieldTemplate.
Implementation is straightforward and correct.
116-116: ArrayFieldTitleTemplate is fine.
No functional or maintainability concerns.
122-122: ArrayFieldTemplate function introduced smoothly.
No immediate issues found.
153-157: Overall structure of ObjectFieldTemplate looks good.
FormContext usage is consistent.
158-163: No concerns with localisableFields extraction.
Logic to locate localizable schema fields is clear.
164-198: Localizable field rendering is well-structured.
Provides a distinct UI block for MDMS & Localization codes.🧰 Tools
🪛 Biome (1.9.4)
[error] 177-179: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
[error] 187-189: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
177-179: Labels must be linked to form inputs or changed to headings.
This repeats a prior comment about accessibility.🧰 Tools
🪛 Biome (1.9.4)
[error] 177-179: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
187-189: Duplicate a11y concern: associate the label with an input.
As previously noted, ensure compliance with label-input pairing.🧰 Tools
🪛 Biome (1.9.4)
[error] 187-189: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
200-200: No additional feedback.
Simple closing brace.
202-202: No changes required.
Field-wrapper usage is consistent.
226-226: No specific changes needed.
CustomFieldTemplate references are correct.
230-239: Title code transformations look fine.
Logic for fallback suffix is clear.
253-253: No issues found.
Children container is well-organized.
263-264: FieldErrorTemplate is concise and clear.
No adjustments needed.
281-281: No concerns withv2prop default.
The approach is straightforward.
285-285: Initializing state with empty object is good practice.
No changes suggested.
289-290: Tenant ID retrieval is consistent with existing patterns.
Implementation is correct.
291-299: Custom MDMS hook usage is correct.
Data selection function is clearly defined.
304-304: Use const instead of let for newAdditionalProps.
Same guidance as the previous review.🧰 Tools
🪛 Biome (1.9.4)
[error] 304-304: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
321-321: No issues at this line.
The effect’s ending brace is properly placed.
335-335: Use Object.hasOwn(...) for safer property checks.
Mirrors the existing comment on avoiding prototype lookups.🧰 Tools
🪛 Biome (1.9.4)
[error] 335-335: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
353-353: Same note: replace hasOwnProperty with Object.hasOwn().
Maintains consistency with best practices.🧰 Tools
🪛 Biome (1.9.4)
[error] 353-353: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
371-371: Use const for locale.
Matches prior suggestion regarding immutability.🧰 Tools
🪛 Biome (1.9.4)
[error] 371-371: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
387-387: Switch to optional chaining.
Matches your existing comment about more idiomatic usage.🧰 Tools
🪛 Biome (1.9.4)
[error] 387-387: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
413-413: Template literal recommended for string concatenation.
This is the same feedback as before.🧰 Tools
🪛 Biome (1.9.4)
[error] 413-413: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
479-486: Make Toast a self-closing element if no children exist.
Consistent with the earlier comment for this line range.🧰 Tools
🪛 Biome (1.9.4)
[error] 479-486: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
244-244: 🧹 Nitpick (assertive)Use template literals for the label id.
Currently:id={"label_" + id}. Improve readability with a template literal.- <label htmlFor={id} className="control-label" id={"label_" + id}> + <label htmlFor={id} className="control-label" id={`label_${id}`}>Likely invalid or redundant comment.
|
|
||
| const onError = (errors) => { | ||
| setLiveValidate(true); | ||
| onFormError(errors); | ||
| onFormError && onFormError(errors); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Leverage optional chaining for onFormError.
Avoid onFormError && onFormError(errors); in favor of onFormError?.(errors);.
- onFormError && onFormError(errors);
+ onFormError?.(errors);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onFormError && onFormError(errors); | |
| onFormError?.(errors); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 394-394: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Show resolved
Hide resolved
| const { data: MdmsRes } = Digit.Hooks.useCustomMDMS( | ||
| tenantId, | ||
| "Workbench", | ||
| [{ name: "UISchema" }], |
There was a problem hiding this comment.
@mithunhegde-egov this is not handled yet ?
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js(11 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
[error] 182-184: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 192-194: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 248-248: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 340-340: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 358-358: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 392-392: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 399-399: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 418-418: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 484-491: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
[error] 309-309: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
[error] 376-376: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🔇 Additional comments (12)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js (12)
14-14: No issues found here.
25-25: Maintain naming consistency.You’re using “tranformLocModuleName” in multiple files. Consider standardizing the name across the codebase to either “transformLocModuleName” or “tranformLocModuleName.”
31-32: Context API usage looks good.
52-54: Commented-out code removal acknowledgement.These comment lines indicate that certain error transformations and references to Digit were removed to simplify the code. No further concerns.
158-162: No immediate issues in ObjectFieldTemplate declarations.
182-184: Label elements without associated inputs.A form label must be associated with an input for proper accessibility. Where these labels are purely descriptive, consider changing them to headings (e.g.,
<h4>or<span>) or linking them to an actual form control with anhtmlForattribute.Also applies to: 192-194
🧰 Tools
🪛 Biome (1.9.4)
[error] 182-184: A form label must be associated with an input.
Consider adding a
fororhtmlForattribute to the label element or moving the input element to inside the label element.(lint/a11y/noLabelWithoutControl)
309-309: Use const instead of let for newAdditionalProps.- let newAdditionalProps = {}; + const newAdditionalProps = {};🧰 Tools
🪛 Biome (1.9.4)
[error] 309-309: This let declares a variable that is only assigned once.
'newAdditionalProps' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
340-340: Replace "hasOwnProperty" with "Object.hasOwn()".- if (additionalProperties.hasOwnProperty(fieldName)) { + if (Object.hasOwn(additionalProperties, fieldName)) {Also applies to: 358-358
🧰 Tools
🪛 Biome (1.9.4)
[error] 340-340: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
376-376: Use const for locale.- let locale = Digit.StoreData.getCurrentLanguage(); + const locale = Digit.StoreData.getCurrentLanguage();🧰 Tools
🪛 Biome (1.9.4)
[error] 376-376: This let declares a variable that is only assigned once.
'locale' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
392-392: Consider optional chaining for callback invocations.- onSubmit && onSubmit(transformedFormData, additionalProperties); + onSubmit?.(transformedFormData, additionalProperties); - onFormError && onFormError(errors); + onFormError?.(errors);Also applies to: 399-399
🧰 Tools
🪛 Biome (1.9.4)
[error] 392-392: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
418-418: Prefer template literals.- ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_` + schema?.code)) : `SCHEMA_${schema?.code}` + ? t(Digit.Utils.workbench.getMDMSLabel(`SCHEMA_${schema?.code}`)) : `SCHEMA_${schema?.code}`🧰 Tools
🪛 Biome (1.9.4)
[error] 418-418: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
484-491: Use a self-closing Toast element.- <Toast - label={t(showToast)} - error={showErrorToast} - onClose={() => { - setShowToast(null); - }} - isDleteBtn={true} - ></Toast> + <Toast + label={t(showToast)} + error={showErrorToast} + onClose={() => setShowToast(null)} + isDleteBtn={true} + />🧰 Tools
🪛 Biome (1.9.4)
[error] 484-491: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Unsafe fix: Use a SelfClosingElement instead
(lint/style/useSelfClosingElements)
micro-ui/web/micro-ui-internals/packages/modules/workbench/src/components/DigitJSONForm.js
Outdated
Show resolved
Hide resolved
…/components/DigitJSONForm.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
jagankumar-egov
changed the title
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
SN-368
Module
workbench
Description
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores